[WIP]: Provider components as artifacts #371
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
approved
|
@randomvariable would this change the development experience from the current workflow? If so, we should probably update the docs or Makefile to make it easier. |
|
@vincepri Both DevX and UserX will change, and I need to document both |
randomvariable
force-pushed
the
provider-components-artifacts
branch
from
1097f94 to
8d656af
Compare
randomvariable
force-pushed
the
provider-components-artifacts
branch
from
8d656af to
6a1a5d2
Compare
randomvariable
force-pushed
the
provider-components-artifacts
branch
from
6a1a5d2 to
e87df1a
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: randomvariable The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| MANAGER_IMAGE_TAG ?= 0.0.3 | ||
| MANAGER_IMAGE ?= $(STABLE_DOCKER_REPO)/$(MANAGER_IMAGE_NAME):$(MANAGER_IMAGE_TAG) | ||
| DEV_DOCKER_REPO ?= gcr.io/$(shell gcloud config get-value project) | ||
| DEV_MANAGER_IMAGE ?= $(DEV_DOCKER_REPO)/$(MANAGER_IMAGE_NAME):$(MANAGER_IMAGE_TAG) |
There was a problem hiding this comment.
Would it make sense to add a comment stating that these variables can be overridden?
Makefile
Outdated
|
|
||
| .PHONY: release-binaries | ||
| release-binaries: ## Build release binaries | ||
| bazel build --platforms=@io_bazel_rules_go//go/toolchain:linux_amd64 //cmd/clusterctl //cmd/clusterawsadm | ||
| bazel build --platforms=@io_bazel_rules_go//go/toolchain:darwin_amd64 //cmd/clusterctl //cmd/clusterawsadm | ||
| bazel build --platforms=@io_bazel_rules_go//go/toolchain:linux_amd64,@io_bazel_rules_go//go/toolchain:darwin_amd64 //cmd/clusterctl //cmd/clusterawsadm |
There was a problem hiding this comment.
Nice! is this new in bazel? Last time I tried to parse the docs it looked like --platforms would only take a singular value.
Makefile
Outdated
| @@ -99,32 +88,33 @@ release-binaries: ## Build release binaries | |||
| test: generate verify ## Run tests | |||
| bazel test --nosandbox_debug //pkg/... //cmd/... $(BAZEL_ARGS) | |||
|
|
|||
| .PHONY: install-dev | |||
There was a problem hiding this comment.
should this be install-dev-binaries instead? It might be a good idea to add dependencies here on the clusterctl and clusterawsadm binaries to ensure that it is installing an up to date build.
There was a problem hiding this comment.
we are using -dev as a suffix, so I'd recommend install-binaries-dev (like docker-push-dev)
| .PHONY: docker-push | ||
| docker-push: generate ## Push production docker image | ||
| docker-push: generate provider-components ## Push production docker image |
There was a problem hiding this comment.
Am I missing the provider-components target? I only see provider-components-dev added
| template: | ||
| spec: | ||
| containers: | ||
| - image: gcr.io/cluster-provider-ssh-rv/cluster-api-aws-controller:0.0.2 |
There was a problem hiding this comment.
This seems like an odd value to have embedded here, should this be templated instead?
randomvariable
force-pushed
the
provider-components-artifacts
branch
from
e87df1a to
acd6dbc
Compare
|
@randomvariable: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
I'm honestly pretty concerned at the amount of changes this PR is carrying. I'd like to make sure we are solving an actual problem and find the right solution. In general, it seems like a very complicated approach without many benefits.
If the goal is to provide components as artifact, can it be added to the release workflow? Could we also split the changes in multiple parts making sure each is self-contained?
|
Superseded by #580 |
What this PR does / why we need it:
Simplifies end-user experience to not require anything other than kustomize, clusterawsadm and clusterctl to stand up a cluster.
Bazel generates a static provider-components.yaml file based on Git tags at release.
TODO:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #299, #276
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: